Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

!!! FEATURE: Upgrade CKEditor 5 from version 16.0.0 to 44.0.0 #3883

Merged
merged 18 commits into from
Dec 13, 2024

Conversation

skurfuerst
Copy link
Member

@skurfuerst skurfuerst commented Nov 10, 2024

This update represents a major leap forward, spanning several years of development. Due to the extensive changes and numerous breaking changes, it’s challenging to provide a comprehensive list of all API alterations. As such, this PR introduces a breaking change.

The CKEditor API has evolved significantly, and any plugins relying on outdated APIs may no longer function as expected. We cannot provide a detailed upgrade guide for your custom or third-party plugins. Instead, we recommend consulting the release notes of CKEditor 5 for the relevant versions. Note that during testing, some packages dependent on the old implementation were found incompatible.

To support the community, we aim to review a few community plugins and, where feasible, submit PRs to update them, offering practical examples.

This upgrade effort has been attempted multiple times over the years, but often proved too challenging to complete. We want to acknowledge and thank those who contributed to earlier PRs addressing this task, including @JamesAlias, @73nici, @skurfuerst , @mhsdesign , and @markusguenther

Testing Notes:
Acceptance tests for editable content elements encountered issues with the new CKEditor. The workaround implemented in this PR leverages TestCafe’s proposed solution, but it only works reliably in Chrome. Fortunately, the TestCafe team has already merged a PR that will likely resolve this issue in the future.

Special thanks to everyone involved in this significant effort!

Please see https://discuss.neos.io/t/feedback-needed-in-which-release-to-include-ckeditor-5-update/6707 for context and discussion.

Resolves: #2836

@skurfuerst skurfuerst added the 8.4 label Nov 10, 2024
@markusguenther markusguenther force-pushed the feature/ckeditor-update branch from 9804d83 to 99ce89b Compare December 2, 2024 20:35
@markusguenther markusguenther changed the base branch from 8.4 to 9.0 December 2, 2024 20:36
@github-actions github-actions bot added 9.0 and removed 8.4 labels Dec 2, 2024
@markusguenther
Copy link
Member

As described in the commits, the new type around buttons are not implemented in the UI yet. The buttons create a paragraph before or after the table. At the moment, we hide the buttons as we miss the styles and the functionality.

TBH, as we can just use ENTER to add a paragraph, I am unsure if we need it.
Screenshot 2024-12-02 at 21 37 17

With the latest commits, I also fixed the table drop-downs. We try to render an image with an SVG as markup in the src attribute. As this does not work and the fallback label was not aligned horizontally. Now we transform the svg to an data-uri and use this.
Screenshot 2024-12-02 at 21 37 02

@markusguenther
Copy link
Member

Will try to check the tests tomorrow. A hint can be maybe that issue DevExpress/testcafe#7642

This was referenced Dec 3, 2024
@markusguenther markusguenther marked this pull request as ready for review December 3, 2024 09:27
This is a workaround for the fact that the contenteditable element is not directly selectable for more information see https://testcafe.io/documentation/402688/reference/test-api/testcontroller/selecteditablecontent
@markusguenther markusguenther force-pushed the feature/ckeditor-update branch 4 times, most recently from 98e27e7 to ebc122b Compare December 4, 2024 16:15
@markusguenther
Copy link
Member

Although I added some workarounds I still have some failing tests in the Syncing Test suites.
I hope that I can resolve them somehow. The patch of testcafe did not work for me.

Hope that they the community finds a solution for the contentEditable issue in testcafe.
DevExpress/testcafe#8321

@markusguenther markusguenther force-pushed the feature/ckeditor-update branch 2 times, most recently from 4211e5c to 22183a7 Compare December 5, 2024 10:08
@mhsdesign
Copy link
Member

mhsdesign commented Dec 5, 2024

when i click on a shortcut this pops up in the bottom (and the publish toolbar is gone):

image

@mhsdesign
Copy link
Member

mhsdesign commented Dec 5, 2024

And i think we should put special focus on these things when testing:

@markusguenther markusguenther force-pushed the feature/ckeditor-update branch 2 times, most recently from a101230 to cd73131 Compare December 5, 2024 14:44
As the firefox could not deal with the testcafe workaround. We now write the content to the CKE
@markusguenther markusguenther force-pushed the feature/ckeditor-update branch from 57ab23a to 0aed581 Compare December 5, 2024 18:41
@markusguenther markusguenther force-pushed the feature/ckeditor-update branch from 6e86c3d to 8034d9d Compare December 5, 2024 19:51
@markusguenther markusguenther force-pushed the feature/ckeditor-update branch from 8034d9d to a1acdf5 Compare December 5, 2024 22:05
@markusguenther
Copy link
Member

For auto-paragraphs we have a test ;)
The placeholders work fine, and I could not reproduce the CKEditor bar from your image.

But I am open for feedback :)
Lucky that the tests are green again with some tweaks for the CKEditor, but hope that testcafe version 3.7.1 comes with a fix. A change has been merged today that promises to solve that 🤞

@markusguenther
Copy link
Member

Found a bug with the table toolbar. When you edit a table and go to another page, the toolbar is empty except the table dropdowns. But the bugs seem familiar to me, so will check if it is just an old one.

@markusguenther markusguenther force-pushed the feature/ckeditor-update branch from 564c269 to e6322a6 Compare December 6, 2024 07:35
@markusguenther markusguenther changed the title CKEditor Update !!!FEATURE: Upgrade CKEditor 5 from version 16.0.0 to 44.0.0 Dec 6, 2024
@markusguenther markusguenther changed the title !!!FEATURE: Upgrade CKEditor 5 from version 16.0.0 to 44.0.0 !!! FEATURE: Upgrade CKEditor 5 from version 16.0.0 to 44.0.0 Dec 6, 2024
@markusguenther markusguenther force-pushed the feature/ckeditor-update branch from d0c9241 to e6322a6 Compare December 6, 2024 08:41
@markusguenther
Copy link
Member

Found a bug with the table toolbar. When you edit a table and go to another page, the toolbar is empty except the table dropdowns. But the bugs seem familiar to me, so will check if it is just an old one.

🙈 Found the matching issue for that #3512

@mhsdesign
Copy link
Member

mhsdesign commented Dec 6, 2024

THANK you so much for your effort!

and I could not reproduce the CKEditor bar from your image.

now its already less big and without branding - i assume because of the licence key thingy?

but still after clicking randomly on document nodes below the appContainer there will be elments which take a couple pixels in height and push the ui upwards:

image

the code looks like

<div class="ck ck-clipboard-drop-target-line ck-hidden"></div>
<div class="ck ck-aria-live-announcer"><div aria-live="polite" aria-relevant="additions"><ul class="ck ck-aria-live-region-list"></ul></div><div aria-live="assertive" aria-relevant="additions"><ul class="ck ck-aria-live-region-list"></ul></div></div>

so i think we need to disable some kind of drag and drop plugin and also this aria-live thing? https://ckeditor.com/docs/ckeditor5/latest/api/module_ui_arialiveannouncer-AriaLiveAnnouncer.html

(tested on firefox and safari)

@markusguenther
Copy link
Member

Thanks, @mhsdesign ... I will take a look. Maybe my screen is too small. Used just the MacBook for the week.

Sometimes we need to overwrite global ck editor styles. We can put them here.
@mhsdesign mhsdesign requested a review from Sebobo December 12, 2024 10:35
markusguenther pushed a commit to markusguenther/neos-ui that referenced this pull request Dec 13, 2024
@markusguenther
Copy link
Member

markusguenther commented Dec 13, 2024

Tested the CKEditor in Drupal and TYPO3 and the powered by is rendered in both OSS CMS.
So maybe we should put into the toolbar or so.

TYPO3 v13
Screenshot 2024-12-13 at 11 06 13

Drupal 10
Screenshot 2024-12-13 at 11 13 12

@markusguenther
Copy link
Member

markusguenther commented Dec 13, 2024

We (Neos 9 weekly meeting) decided to merge this, and if we have issues and/or future adjustments that are needed to be done as a follow-up.

@markusguenther markusguenther merged commit 9d9d5a3 into 9.0 Dec 13, 2024
10 checks passed
@markusguenther markusguenther deleted the feature/ckeditor-update branch December 13, 2024 11:52
@jonnitto
Copy link
Member

Thank you all for your efforts and work. Awesome! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

!!! FEATURE: Upgrade CKEditor 5 from version 16.0.0 to 44.0.0
4 participants